Migrate device table to TanStack Table v9 by bdraco · Pull Request #1660 · esphome/device-builder-frontend

bdraco · 2026-08-18T23:28:25Z

What does this implement/fix?

Takes the TanStack Table v9 major that dependabot proposed in #1649; v9 is a breaking rewrite, so the bump alone fails typecheck and tests. The row model factories move into a shared tableFeatures object (new table-features.ts), VisibilityState becomes ColumnVisibilityState, column defs use sortFn, and the table types gain the features generic. Behavior is unchanged; sorting, filtering, column visibility and pagination keep the same controlled state flow.

Supersedes #1649.

Related issue or feature (if applicable):

supersedes Bump @tanstack/lit-table from 8.21.3 to 9.1.2 #1649

Types of changes

Bugfix (non-breaking change which fixes an issue) — bugfix
New feature (non-breaking change which adds functionality) — new-feature
Enhancement to an existing feature — enhancement
Breaking change (fix or feature that would cause existing functionality to not work as expected) — breaking-change
Refactor (no behaviour change) — refactor
Documentation only — docs
Maintenance / chore — maintenance
CI / workflow change — ci
Dependencies bump — dependencies

Checklist

The code change is tested and works locally.
pnpm run lint passes.
pnpm run test passes.
Tests have been added to verify that the new code works (where applicable).

bdraco · 2026-08-18T23:28:39Z

@esphbot review

Copilot

Pull request overview

Migrates the device table to TanStack Table v9 while preserving controlled sorting, filtering, visibility, and pagination behavior.

Changes:

Upgrades @tanstack/lit-table to v9.1.2.
Introduces a shared v9 feature configuration and composed table types.
Updates table implementation and tests for v9 APIs and renamed types.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`package.json`	Upgrades TanStack Lit Table.
`pnpm-lock.yaml`	Locks v9 and its dependencies.
`src/components/dashboard/table-features.ts`	Defines shared table features and types.
`src/components/dashboard/device-table.ts`	Migrates table state and controller APIs.
`src/components/dashboard/device-table-grid.ts`	Uses composed v9 table types.
`src/components/dashboard/table-columns.ts`	Migrates column definitions and sort functions.
`src/components/dashboard/render-content.ts`	Updates visibility event typing.
`src/components/dashboard/prefs.ts`	Updates preference state typing.
`src/pages/dashboard.ts`	Updates dashboard visibility state typing.
`test/components/dashboard/table-columns.test.ts`	Migrates cell-context test types.
`test/components/dashboard/prefs.test.ts`	Migrates visibility-state test types.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

esphbot · 2026-08-19T00:06:42Z

~~Previous review~~ — superseded by a newer review below.

esphbot

Tip

No blocking issues found — ready to merge.

bdraco · 2026-08-19T00:10:43Z

Both suggestions addressed in 270c207. Added a test pinning the auto resolved alphanumeric order on the platform column; the fixture includes host2/host10 and a case mixed value so a silent basic fallback fails the assertion rather than passing with the same order. Tightened the table-features docstring to name the unregistered datetime and basic outcomes. Moved DeviceRow into a leaf device-row.ts so table-features and table-columns no longer import each other; table-columns re-exports the type, so call sites are unchanged.

bdraco · 2026-08-19T00:12:57Z

@esphbot review

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

Suppressed comments (1)

src/components/dashboard/table-features.ts:42

The new test only forces auto to choose alphanumeric because its sampled values contain digits. Removing the separately registered text comparator would therefore leave this test green, even though plain-string columns such as Area and Comment would fall back to case-sensitive basic sorting. Please add a plain-string case whose expected order distinguishes text from basic so both entries in the slim registry are covered.

  sortFns: { alphanumeric: sortFn_alphanumeric, text: sortFn_text },

bdraco · 2026-08-19T01:15:53Z

@esphbot review

esphbot · 2026-08-19T01:16:17Z

@bdraco: Both suggestions addressed in 270c207. Added a test pinning the auto resolved alphanumeric order on...

Verified all three on 270c207b with v9.1.2 installed. Platform test passes and fails under both degraded paths — host2/host10 breaks basic, and text too. Cycle gone; table-columns.ts re-export keeps call sites intact. tsc, eslint, prettier clean; test/components 381 files / 3141 tests pass.

One gap remains, non-blocking. Test pins alphanumeric only. Dropping text leaves it green. Mounted address with Zeta/apple/Banana — renders apple, Banana, Zeta, so text resolves for digit-free columns; basic would give Banana, Zeta, apple. text also backstops an alphanumeric miss in column_getAutoSortFn. One extra case in that describe covers it.

esphbot · 2026-08-19T01:16:19Z

PR Review — Migrate device table to TanStack Table v9

Both prior suggestions are genuinely addressed. Verified against the real v9 packages — merge-ready with one small coverage nit.

Specifically good in this round:

The new sort test actually discriminates. The fixture is chosen with care: host2/host10 breaks a basic fallback and a text fallback, BK72XX covers the case dimension. I ran it on a fresh checkout with @tanstack/lit-table@9.1.2 installed — it passes, and I confirmed by reading column_getAutoSortFn that both degraded paths would produce a different order rather than coincidentally the same one. It pins behavior, not implementation.
The DeviceRow move is a clean cycle break. device-row.ts is type-only, table-columns.ts re-exports it (export type { DeviceRow }), so every existing call site — including test/components/dashboard/table-columns.test.ts — is untouched, and table-features.ts no longer points back at table-columns.ts. The runtime-value TDZ hazard I flagged is gone.
The docstring now names the failure mode, including the dev-only console.warn and the sortFn_datetime requirement for a future Date column. That matches what the package actually does.

Verification on 270c207b with v9 installed: tsc --noEmit exit 0, eslint src test --max-warnings=0 exit 0, prettier --check clean, test/components 381 files / 3141 tests all passing, and the three directly-touched test files pass. I also mounted the table by hand and confirmed the search box still filters rows through v9's global filtering (search = "beta" → one row) and that a digit-free column sorts case-insensitively via text.

Suggestion: the new test locks alphanumeric but not text; removing text from the registry silently makes address / comment / area / status case-sensitive with a green suite. A digit-free mixed-case case in the same describe closes it.
Suggestion: device-row.ts splits two type imports of api/types/devices.js across two statements — collapse to one.

🟢 Suggestions

1. New sort test pins `alphanumeric` only — dropping `text` from the registry stays green
test/components/dashboard/device-table.test.ts:142-169

The new test closes the gap I raised last round for alphanumeric, and it discriminates correctly: I ran it against v9.1.2 and confirmed host2 before host10 fails under a basic fallback, and under a text fallback too (text lowercases but still puts host10 first). So removing alphanumeric is caught.

What is still unpinned is the other half of the registry. text is load-bearing here for two separate reasons, and neither is covered:

It is what digit-free string columns resolve to. I mounted the table on this branch with address values Zeta.local / apple.local / Banana.local and initialSorting: [{id: "address"}]; the rendered order was apple, Banana, Zeta — case-insensitive, i.e. sortFn_text. Drop text from the registry and column_getAutoSortFn returns sortFn_basic, giving Banana, Zeta, apple. Same exposure for comment, area, config and status whenever their sampled values contain no digits. The platform fixture all contains digits or resolves before text is consulted, so it stays green through that change.
It is also the secondary fallback for alphanumeric. In rowSortingFeature.utils.js, an alphanumeric miss retries sortFns?.text before falling through to basic — so text backstops the entry the new test does cover.

Concrete fix: add a second case to the same describe sorting a digit-free column (address is convenient — visible by default, plain valueCell) over mixed-case values, asserting apple.local, Banana.local, Zeta.local. That order distinguishes text from basic and from textCaseSensitive. This is the same point Copilot raised and self-suppressed; I verified it holds against the real package.

Non-blocking — the registry is correct as shipped.

expect(order).toEqual(["BK72XX", "esp32", "esp32-c3", "host2", "host10", "rp2040"]);

Checklist

Important

The branch moved during review. This review was performed against HEAD=270c207, but the PR branch now points at c0f1e39. Commits pushed after the review started are not reflected below — re-run /review to cover them.

Automated review by Kōan (Claude) HEAD=270c207 10 min 24s

esphbot

Tip

No blocking issues found — ready to merge.

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

bdraco · 2026-08-19T01:22:38Z

Suggestion 1 was already addressed in c0f1e39, which the review notes it could not see; it pins the text entry by sorting the digit-free comment column over kitchen, Bedroom, attic and asserting attic, Bedroom, kitchen, an order that fails under both basic and textCaseSensitive. Suggestion 2 is fixed in the commit above, the two device-row type imports are now one statement.

@esphbot review

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (1)

pnpm-lock.yaml: Generated file

bdraco · 2026-08-19T02:19:46Z

@esphbot review

Migrate device table to TanStack Table v9

b4d2c86

bdraco added the dependencies Dependency updates label Aug 18, 2026

Slim the v9 sort registry and share the table type aliases

cdfb3ad

bdraco requested a balanced review from Copilot August 18, 2026 23:47

bdraco marked this pull request as ready for review August 18, 2026 23:47

Copilot started reviewing on behalf of bdraco August 18, 2026 23:47 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

esphbot approved these changes Aug 19, 2026

View reviewed changes

Pin the auto sort order and break the table type import cycle

270c207

Copilot AI review requested due to automatic review settings August 19, 2026 00:10

Copilot started reviewing on behalf of bdraco August 19, 2026 00:11 View session

Copilot AI reviewed Aug 19, 2026

View reviewed changes

Pin the text sort entry with a digit-free column test

c0f1e39

Copilot AI review requested due to automatic review settings August 19, 2026 01:15

esphbot approved these changes Aug 19, 2026

View reviewed changes

Copilot started reviewing on behalf of bdraco August 19, 2026 01:16 View session

Copilot AI reviewed Aug 19, 2026

View reviewed changes

Collapse the device-row type imports into one statement

a11f148

Copilot AI review requested due to automatic review settings August 19, 2026 01:22

Copilot started reviewing on behalf of bdraco August 19, 2026 01:23 View session

Copilot AI reviewed Aug 19, 2026

View reviewed changes

bdraco merged commit 3738dc3 into main Aug 19, 2026
14 checks passed

bdraco deleted the tanstack-table-v9 branch August 19, 2026 01:26

esphome Bot mentioned this pull request Aug 19, 2026

Bump frontend to 0.1.307 esphome/device-builder#2597

Merged

9 tasks

esphbot mentioned this pull request Aug 19, 2026

Retire the quiet-serial banner once the console has produced output #1661

Merged

13 tasks

github-actions Bot locked and limited conversation to collaborators Aug 21, 2026

Uh oh!

Uh oh!

Conversation

bdraco commented Aug 18, 2026

What does this implement/fix?

Types of changes

Checklist

Uh oh!

bdraco commented Aug 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

esphbot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

esphbot left a comment

Choose a reason for hiding this comment

Uh oh!

bdraco commented Aug 19, 2026

Uh oh!

bdraco commented Aug 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bdraco commented Aug 19, 2026

Uh oh!

esphbot commented Aug 19, 2026

Uh oh!

esphbot commented Aug 19, 2026

PR Review — Migrate device table to TanStack Table v9

🟢 Suggestions

Checklist

Uh oh!

esphbot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bdraco commented Aug 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

bdraco commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

esphbot commented Aug 19, 2026 •

edited

Loading